[VW-319] Adding teamplay Fleet Work Order Integration#152
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
The bulk of the teamplay Fleet to VIPER work order mapping code
| map: (raw, url) => | ||
| mapFleetActivities(raw, { offset: deriveOffsetFromUrl(url) }), | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
This is where we can expand easily to other partner integrations with similar workflows.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/tracking/server/fleet-mapper.ts (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
server-onlydirective to enforce server/client boundary.This file lives in
src/features/tracking/server/and uses kebab-case utility naming, making it a server utility file per the project's naming convention. It should be marked withserver-onlyto prevent accidental client-side imports.As per coding guidelines: "Mark server utility files with 'server-only' and client components with 'use client' directive to enforce clear server/client boundaries."
🔧 Add server-only directive
+import "server-only"; import { z } from "zod"; import { NotificationChannel, TicketCategory, TicketStatus, } from "`@/generated/prisma`";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/tracking/server/fleet-mapper.ts` around lines 1 - 6, Add the server-only boundary marker to this server utility module so it cannot be imported from client code. Update the top of the file in fleet-mapper (the module that imports zod and Prisma enums) to include the server-only directive before other imports, keeping the rest of the server-side logic unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/inngest/functions/sync-integrations.ts`:
- Around line 225-236: The hostname lookup in selectRestMapper is too permissive
because host.includes(m.hostMatch) can match lookalike domains and choose the
wrong REST_MAPPER. Update selectRestMapper to use exact host matching or safe
subdomain-suffix matching against integration.integrationUri’s parsed hostname,
and keep the resourceType check intact so only the intended adapter is returned.
---
Nitpick comments:
In `@src/features/tracking/server/fleet-mapper.ts`:
- Around line 1-6: Add the server-only boundary marker to this server utility
module so it cannot be imported from client code. Update the top of the file in
fleet-mapper (the module that imports zod and Prisma enums) to include the
server-only directive before other imports, keeping the rest of the server-side
logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9f255588-9774-4020-8798-7cbe80ce871c
📒 Files selected for processing (12)
prisma/migrations/20260708073706_work_order_integration_squashed/migration.sqlprisma/schema.prismasrc/features/api-key-connectors/components/connectors-layout.tsxsrc/features/integrations/components/integrations.tsxsrc/features/integrations/types.tssrc/features/tag-colors/components/tag-colors.tsxsrc/features/tracking/components/ticket-detail/shared.tsxsrc/features/tracking/server/fleet-mapper.test.tssrc/features/tracking/server/fleet-mapper.tssrc/features/tracking/server/routers.tssrc/features/tracking/types.tssrc/inngest/functions/sync-integrations.ts
| function selectRestMapper(integration: IntegrationWithStringDates) { | ||
| let host = ""; | ||
| try { | ||
| host = new URL(integration.integrationUri).hostname.toLowerCase(); | ||
| } catch { | ||
| // Malformed URL → no adapter matches; the caller throws a clear error. | ||
| } | ||
| return REST_MAPPERS.find( | ||
| (m) => | ||
| m.resourceType === integration.resourceType && host.includes(m.hostMatch), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
selectRestMapper substring match can select the wrong adapter for lookalike hostnames.
host.includes(m.hostMatch) matches any hostname containing the substring, so a URL like https://fleet.siemens-healthineers.com.evil.com/ would incorrectly select the Fleet adapter. Use exact host or subdomain suffix matching instead.
🔒 Proposed fix for hostname matching
return REST_MAPPERS.find(
(m) =>
- m.resourceType === integration.resourceType && host.includes(m.hostMatch),
+ m.resourceType === integration.resourceType &&
+ (host === m.hostMatch || host.endsWith(`.${m.hostMatch}`)),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function selectRestMapper(integration: IntegrationWithStringDates) { | |
| let host = ""; | |
| try { | |
| host = new URL(integration.integrationUri).hostname.toLowerCase(); | |
| } catch { | |
| // Malformed URL → no adapter matches; the caller throws a clear error. | |
| } | |
| return REST_MAPPERS.find( | |
| (m) => | |
| m.resourceType === integration.resourceType && host.includes(m.hostMatch), | |
| ); | |
| } | |
| function selectRestMapper(integration: IntegrationWithStringDates) { | |
| let host = ""; | |
| try { | |
| host = new URL(integration.integrationUri).hostname.toLowerCase(); | |
| } catch { | |
| // Malformed URL → no adapter matches; the caller throws a clear error. | |
| } | |
| return REST_MAPPERS.find( | |
| (m) => | |
| m.resourceType === integration.resourceType && | |
| (host === m.hostMatch || host.endsWith(`.${m.hostMatch}`)), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/inngest/functions/sync-integrations.ts` around lines 225 - 236, The
hostname lookup in selectRestMapper is too permissive because
host.includes(m.hostMatch) can match lookalike domains and choose the wrong
REST_MAPPER. Update selectRestMapper to use exact host matching or safe
subdomain-suffix matching against integration.integrationUri’s parsed hostname,
and keep the resourceType check intact so only the intended adapter is returned.
2 participants
Adding new Integration workflow for teamplay Fleet
to replicate, add the following Work Order integration
URL: https://fleet.siemens-healthineers.com/rest/v1/activities?tz=-05%3A00&statusFilter=1
Auth:
HeaderHeader Name:
CookieHeader Value:
XSRF-TOKEN={value}; subTenantClientId={value}; JSESSIONID={value};Summary by CodeRabbit
New Features
Bug Fixes